Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use tmpfs for etcd in e2e test clusters #180

Merged
merged 2 commits into from
Jul 24, 2023
Merged

Conversation

pablochacin
Copy link
Collaborator

@pablochacin pablochacin commented Jun 3, 2023

Description

Add configuration to the kind cluster for setting etcd storage path to a memory-mapped volume.

Fixes #179

Checklist:

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works.
  • I have run linter locally (make lint) and all checks pass.
  • I have run tests locally (make test) and all tests pass.
  • I have run relevant e2e test locally (make e2e-xxx for agent, disruptors, kubernetes or cluster related changes)
  • Any dependent changes have been merged and published in downstream modules

@pablochacin
Copy link
Collaborator Author

Initial tests show no significant difference running disruptor e2e tests:

go test -tags e2e e2e/disruptors/pod_e2e_test.go

Before

ok  	command-line-arguments	137.136s

After

ok  	command-line-arguments	133.973s

This can be due to the disk not being a bottleneck in the test machine (which uses SSD)

@nadiamoe
Copy link
Member

nadiamoe commented Jun 6, 2023

Even if there's not a big improvement, it seems nicer on the environment (by means of less trashed SSDs :P)
It would be nice to have this by default, specially if we don't foresee etcd to grow big enough to eat the whole RAM.

@pablochacin pablochacin marked this pull request as ready for review July 21, 2023 13:39
@pablochacin pablochacin requested a review from nadiamoe July 21, 2023 13:40
@nadiamoe
Copy link
Member

How can I check if this is working?

I've tried running E2E_ETCD_RAMDISK=1 make e2e-disruptors, but I'm not seeing what I would expect. By describing the etcd pod in the cluster that gets created, I can see:

  volumes:
    - hostPath:
        path: /etc/kubernetes/pki/etcd
        type: DirectoryOrCreate
      name: etcd-certs
    - hostPath:
        path: /var/lib/etcd
        type: DirectoryOrCreate
      name: etcd-data

If I exec into the kindest/node, /var/lib/etcd is not mounted as tmpfs. From the code I guess path for the etcd-data volume should be /tmp/etcd instead. Am I doing something wrong?

@pablochacin
Copy link
Collaborator Author

Am I doing something wrong?

E2E_ETCD_RAMDISK=1 should be E2E_ETCD_RAMDISK=true or be omitted because it is the default.

@pablochacin
Copy link
Collaborator Author

pablochacin commented Jul 21, 2023

@roobre You were right, I had a bug and wasn't passing the option from the e2e cluster config to the kind cluster config. Good catch.

I fixed it and checked that with the default settings the etcd pod mounts the etcd-data volume at /tmp/etcd and that the tmp directory is mounted on a tmpfs file system in the host node.

Copy link
Member

@nadiamoe nadiamoe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏻 I can see the expected volume in etcd now, LGTM. I also like this being on by default (otherwise I would surely forget to turn it on 😅)

Thanks for taking care of it!

@pablochacin pablochacin merged commit 179a547 into main Jul 24, 2023
@pablochacin pablochacin deleted the use-in-memory-etcd branch July 24, 2023 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use memory storage for etcd in e2d2 test clusters
2 participants